Skip to content

serve --long-model: short conversations on -m's route, long ones on W4A4 - #203

Merged
bong-water-water-bong merged 1 commit into
mainfrom
long-route
Sep 28, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
long-route

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Follow-up to #202. That PR showed decode wins short prompts (Vulkan) and prefill wins long ones (W4A4, from about 2K tokens). This one serves both from one endpoint.

  • 1bit serve -m MODEL.gguf --long-model MODEL-Q4_0-H32.gguf [--long-from 2048] loads both. A conversation whose first request has at least --long-from prompt tokens goes to the rotated file on ROCm W4A4, and everything else goes to -m's route.
  • Prompt tokens are counted by the -m backend (/apply-template + /tokenize, tools included). If that fails, it falls back to about 4 bytes per token.
  • Routing is sticky per conversation, so the prompt cache stays useful as a chat grows. X-1bit-Route: short|long <tokens> <device> is set on every response.
  • It is refused with an unrotated --long-model, a rotated -m, a non-llama-server -m route, and with --laya/--adaptive/--mmproj.
  • tests/long_route.sh (ctest, CI): fake backends, 9 checks. Short and long chats and completions route correctly, a conversation that grows stays put, and the refusals work. hadamard_route and smoke_serve still pass.

Strix Halo, Qwen3-Coder-30B-A3B, 256 output tokens, effective tok/s (tools/e2e_bench.py):

Prompt 512 2,048 8,192 16,384
Vulkan Q4_K_M 70.7 52.1 20.3 8.6
ROCm W4A4 66.6 55.0 27.0 13.4
--long-model 71.3 53.1 25.5 13.1

The --long-model run shared the box with a peer's NPU job, which cost the long side 2-5%. A first run showed 51.6 decode on short prompts. It didn't reproduce in two isolated reruns (79.3-79.8, the same as standalone Vulkan), so it was box contention.

🤖 Generated with Claude Code

The same model served both ways: -m on its usual route (Vulkan) and a
Hadamard-rotated Q4_0 of it on ROCm W4A4. A conversation whose first request
has --long-from prompt tokens or more (default 2048, counted by the backend's
/apply-template + /tokenize) goes to the rotated file; conversations are
sticky so their prompt cache stays useful. X-1bit-Route tells which.

Qwen3-Coder-30B-A3B, effective tok/s at 512/2K/8K/16K prompt tokens:
Vulkan 70.7/52.1/20.3/8.6, W4A4 66.6/55.0/27.0/13.4, --long-model
71.3/53.1/25.5/13.1. tests/long_route.sh covers the routing without a GPU.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@context7

context7 Bot commented Sep 28, 2026

Copy link
Copy Markdown

Docs7 for 1bit-monster/engine

Result Status Action
Deployment ➖ Not used —
Content review ➖ Did not run. This site has no agent runs available this month. Wait for the monthly reset or check your Docs7 plan. —

Commit 60ee749

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

202 - Partially compliant

Compliant requirements:

  • End-to-end benchmarking tool (tools/e2e_bench.py) is implemented and used to measure TTFT, decode, end-to-end, and effective speed
  • Benchmark results for Qwen3-Coder-30B-A3B with different prompt lengths are documented in docs/lean.md and docs/serve.md
  • The implementation shows that Vulkan decodes faster for short prompts and W4A4 prefill wins for long prompts
  • --long-model feature is implemented in 1bit serve to route short and long conversations to different backends
  • Routing is sticky per conversation and prompt cache stays useful
  • Tests for the new routing functionality are added in tests/long_route.sh
  • Documentation is updated to reflect the new feature and benchmark results

Non-compliant requirements:

  • The PR description mentions "effective tok/s" but doesn't explicitly confirm that the numbers in the table match the actual measurements from the hardware

Requires further human verification:

  • The claim that the --long-model implementation achieves better effective tok/s than individual routes needs verification against actual hardware measurements
  • The specific performance gains mentioned in the documentation should be confirmed with real hardware benchmarks
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential race condition in long route caching

The long route caching mechanism uses a mutex (long_mu) to protect access to long_cache and long_order, but there's a potential race condition in the logic where a conversation's first request might be processed by one thread while another thread is still determining the route. This could lead to inconsistent routing decisions for the same conversation.

    const size_t key = conversation_key(q);
    std::pair<size_t, long> side{0, -1};
    bool fresh = true;
    {
        std::lock_guard<std::mutex> lock(long_mu);
        if (const auto c = long_cache.find(key); c != long_cache.end()) side = c->second, fresh = false;
    }
    if (fresh) {
        side.second = prompt_tokens(backends[0].port, q);
        side.first = side.second >= o.long_from ? 1 : 0;
        std::lock_guard<std::mutex> lock(long_mu);
        if (long_cache.size() >= 4096) {  // oldest conversation first
            long_cache.erase(long_order.front());
            long_order.pop_front();
        }
        if (long_cache.emplace(key, side).second) long_order.push_back(key);
    }
    const size_t i = side.first;
    char route_hdr[96];
    std::snprintf(route_hdr, sizeof route_hdr, "%s %ld %s", i ? "long" : "short", side.second, backends[i].device.c_str());
    held = std::make_shared<InFlight>(&inflight[i]);
    ++routed[i];
    forward(backends[i].port, held, -1, id, backends[i].drop_model, backends[i].set_model, q, r);
    r.set_header("X-1bit-Route", route_hdr);  // short or long, the first prompt's tokens, device
    return;
}
Inconsistent error handling in prompt_tokens function

The prompt_tokens function has multiple fallback mechanisms when parsing fails, but the error handling is inconsistent. It catches exceptions in some cases but not others, which could lead to unexpected behavior or missed errors when counting prompt tokens.

long prompt_tokens(int port, const httplib::Request& q) {
    json body;
    try {
        body = json::parse(q.body);
    } catch (const std::exception&) {
        return long(q.body.size() / 4);
    }
    httplib::Client c("127.0.0.1", port);
    c.set_read_timeout(std::chrono::seconds(30));
    std::string prompt;
    if (body.contains("prompt") && body["prompt"].is_string()) {
        prompt = body["prompt"].get<std::string>();
    } else if (auto t = c.Post("/apply-template", q.body, "application/json"); t && t->status == 200) {
        try {
            prompt = json::parse(t->body).value("prompt", "");
        } catch (const std::exception&) {
        }
    }
    if (prompt.empty()) return long(request_state(q).size() / 4);
    if (auto t = c.Post("/tokenize", json{{"content", prompt}}.dump(), "application/json"); t && t->status == 200) {
        try {
            const json tok = json::parse(t->body);
            if (tok.contains("tokens") && tok["tokens"].is_array()) return long(tok["tokens"].size());
        } catch (const std::exception&) {
        }
    }
    return long(prompt.size() / 4);
}
Test relies on specific environment variables

The test script relies on specific environment variables (GGML_Q4_0_HADAMARD, GGML_W4A4_TENSORS) to verify the correct backend is being used, but these are not consistently set or checked in all scenarios, potentially leading to false positives in test results.

env -u GGML_Q4_0_HADAMARD -u GGML_W4A4_TENSORS RECORDS="$scratch/rec" ONEBIT_LEAN_SERVER="$scratch/backend.py" \
    "$bin" serve -m "$scratch/plain.gguf" --long-model "$scratch/h32.gguf" --long-from 100 --port "$port" \

@bong-water-water-bong
bong-water-water-bong merged commit 7396075 into main Sep 28, 2026
11 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the long-route branch September 28, 2026 19:01
bong-water-water-bong added a commit that referenced this pull request Nov 5, 2026
…1073, which re-bumps an already-merged pin) (#1075)

* third_party: bump llama.cpp to 6e42864a6, the 3 fork commits merged since 817d0815b

817d0815ba -> 6e42864a6 (fast-forward on 1bit/hrx-vulkan-patched), three commits:

  6e42864a6  convert/model: store Zamba2's shared transformer block once per block (engine #1036, fork #203)
  b390c2db1  hrx: log a residency decline, so a budget that reshapes the graph is visible (engine #674, fork #174)
  c08e9510b  src: opt-in per-layer attention q/k dump (LLAMA_ZHA_DUMP_DIR) (fork #179)

None touches the architecture registry, so unlike #1039 there is no registry_build/device_route
consequence (checked: 0 files under registry/ in the three commits). Each commit carries its own build
wiring: #203 and #174 only touch existing files plus one new header, and #179 adds src/llama-zha-dump.{h,cpp}
together with its src/CMakeLists.txt entry.

Behaviour a reader bumping this pin should know about:

  * #203 changes what a converted Zamba2 GGUF contains -- a re-convert now writes the shared transformer
    block once per block and the loader aliases the rest (ffn_up stays per layer because of the per-layer
    MLP adapter). An existing Zamba2 file keeps working on the old path; a new one is smaller.
  * #174 adds the hrx placement: decline log (GGML_HRX_HOST_WEIGHT_LIMIT_MB), eight distinct (weight, op)
    pairs then one suppression line, plus a verbosity note in the comment.
  * #179 is gated on LLAMA_ZHA_DUMP_DIR and does nothing when it is unset.

Closes #1073 as superseded: that PR bumps to 817d0815ba, which main already has (#1039, 518cd93), so its
merge was a no-op.

* registry: record the llama.cpp pin at 6e42864a6 after the bump

tools/registry_build.py --check-pins failed the bump (registry_pins, test 43): a pin moved without
regenerating the registry.  Ran it with third_party/llama.cpp at 6e42864a6: hrx 297, npu 7,
architectures 297, mapped 297 -- unchanged from the recorded count, so the three commits add no
architecture.  The only change is the recorded pin:

  -  "llama.cpp (hrx)": "817d0815ba5af..."
  +  "llama.cpp (hrx)": "6e42864a628af..."

--check-pins: "records the committed pins";  --check-gaps: "none of the 28 reviewed significant
classes is mapped without a reason".

---------

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant